Conversation
개요음성 및 텍스트 기반 감정 융합 로직이 향상되었습니다. 변경사항
예상 코드 리뷰 노력🎯 3 (중간) | ⏱️ ~20분
관련 가능성 있는 PR
제안 리뷰어
시
Pre-merge checks and finishing touches❌ Failed checks (2 warnings)
✅ Passed checks (1 passed)
✨ Finishing touches
🧪 Generate unit tests (beta)
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 1
🧹 Nitpick comments (3)
app/services/va_fusion.py (2)
202-227:compute_entropy()반환값을 [0,1]로 클램프하는 방어 로직을 추가 권장합니다.
부동소수/eps 처리로 이론상 1을 아주 미세하게 초과할 여지가 있어, 하위 로직(임계치 비교) 안정성이 좋아집니다.def compute_entropy(probs: Dict[str, float]) -> float: @@ - return h / max_h if max_h > 0 else 0.0 + val = (h / max_h) if max_h > 0 else 0.0 + return max(0.0, min(1.0, val))
327-346: 충돌/엔트로피 기반 neutral 억제 로직:conflict_factor=0.1은 효과는 크지만 급격합니다(경계 테스트 권장).
특히v_audio또는v_text가 노이즈로 부호가 튀는 케이스에서 neutral이 과소평가될 수 있어, 최소 보호장치(예:abs(v_audio)/abs(v_text)가 일정 값 이상일 때만 conflict 적용)도 고려해볼 만합니다.- is_conflict = (v_audio * v_text) < 0 + # 노이즈성 부호 반전을 줄이기 위한 게이트(임계치는 데이터로 조정 권장) + is_conflict = (v_audio * v_text) < 0 and abs(v_audio) >= 0.15 and abs(v_text) >= 0.15app/voice_service.py (1)
325-326: 타임아웃 로그가 항상 “after 30s”로 찍혀 실제remaining과 불일치할 수 있습니다.
운영/장애 분석을 위해 실제 경과 시간을 함께 로깅하는 편이 안전합니다.- except asyncio.TimeoutError: - print(f"STT 타임아웃: voice_id={voice_id} after 30s") + except asyncio.TimeoutError: + elapsed = 30.0 - max(0.0, deadline - time.monotonic()) + print(f"STT 타임아웃: voice_id={voice_id} elapsed={elapsed:.2f}s (deadline=30s)") logger.log_step("stt 타임아웃", category="async")
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (2)
app/services/va_fusion.py(4 hunks)app/voice_service.py(2 hunks)
🧰 Additional context used
📓 Path-based instructions (2)
**/*.py
📄 CodeRabbit inference engine (.cursorrules)
**/*.py: Always useDepends(get_db)to inject database sessions into FastAPI endpoints; never usenext(get_db())as it causes connection leaks by preventing the finally block from executing
Never directly instantiate Service or Repository in endpoints; always use FastAPI'sDepends()for dependency injection with dependency functions namedget_{service_name}_deporget_{repo_name}_dep
Never create database sessions directly usingSessionLocal(); sessions must only be created through theget_dbdependency function at the endpoint level
Useasync deffor all FastAPI endpoint functions
Define explicitresponse_modelparameter in FastAPI endpoints using Pydantic models
Organize imports in the following order: (1) standard library, (2) third-party libraries, (3) local modules
Usesnake_casefor endpoint function names,PascalCasefor class names, andUPPER_SNAKE_CASEfor constants
Add docstrings to all functions and complex logic comments in the code
Store sensitive information such as database credentials in environment variables, never hardcode them
Use custom exceptions inheriting fromAppExceptionbase class for consistent error handling and response formatting
Use global exception handlers to catchSQLAlchemyErrorand ensure consistent error response format for database errors
Use type hints on all function parameters and return values
UseOptional[Type]for nullable values and explicitly handleNonecases in code
Use Pydantic models for all request and response data transfer objects (DTOs)
Setautocommit=Falseand explicitly callcommit()for database transactions; callrollback()on exception to ensure data integrity
Use dependency function chaining: Session (top-level) → Service/Repository → endpoint injection
UseAnnotatedwithDependsfor complex dependency scenarios to improve code clarity and reduce repetition
Use streaming for large file processing to avoid excessive memory consumption
Monitor and log memory usage u...
Files:
app/voice_service.pyapp/services/va_fusion.py
**/services/**/*.py
📄 CodeRabbit inference engine (.cursorrules)
**/services/**/*.py: Define dependency functions in service and repository files or in a dedicateddependencies.pyfile with explicit type hints for all Service and Repository dependencies
Business logic and Service functions must be located in theservices/directory and receive database sessions through constructor parameters or function parameters, not by direct creation
Files:
app/services/va_fusion.py
⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (1)
- GitHub Check: Agent
🔇 Additional comments (3)
app/services/va_fusion.py (2)
1-1:logimport 추가는 적절합니다(새 엔트로피 계산에 사용).
다만 현재 파일에서는log가compute_entropy()에서만 쓰이므로, 해당 함수가 실제로 호출되는 경로가 프로덕션에서 보장되는지만 확인해 주세요.
257-264:angry/fear를 동일한 부정 가중치로 맞춘 변경은 목적(“안정” 과다)을 고려하면 합리적입니다.
다만sad/angry/fear가 동일해지면서 “부정 감정 세부 분리”가 오히려 음성 확률에 더 의존하게 될 수 있으니, 기대 동작(예: angry 과다 top 방지)이 실데이터에서 맞는지 확인이 필요합니다.app/voice_service.py (1)
301-305: 백그라운드 비동기 작업에서는SessionLocal()직접 생성이 적절합니다.현재 구현은 올바릅니다. 가이드라인의 "SessionLocal 직접 생성 금지"는 FastAPI 엔드포인트에서
Depends(get_db)를 사용해야 한다는 의미이며, 백그라운드 작업은Depends()를 사용할 수 없으므로SessionLocal()+try/finally패턴이 정확한 구현입니다. 현재 코드는finally에서db.close()를 호출하고 예외 시db.rollback()도 처리하고 있어 세션 누수 위험이 없습니다.
| try: | ||
| logger.log_step("(비동기 작업) STT 작업 시작", category="async") | ||
| deadline = time.monotonic() + 20.0 | ||
| deadline = time.monotonic() + 30.0 |
There was a problem hiding this comment.
🧩 Analysis chain
🏁 Script executed:
# First, let's check if the file exists and get its context
fd "voice_service.py" -type fRepository: safori-team/CARING-Back
Length of output: 237
🏁 Script executed:
# Read the relevant section around lines 306 and 319
cat -n app/voice_service.py | sed -n '300,330p'Repository: safori-team/CARING-Back
Length of output: 1699
STT→NLP 전체 데드라인을 30초로 늘린 변경은 타당하나, 주석이 여전히 "20초 내"로 남아 있어 혼동을 야기합니다.
Line 319의 주석 (전체 stt->nlp 20초 내)을 (전체 stt->nlp 30초 내)로 갱신해 주세요.
🤖 Prompt for AI Agents
In app/voice_service.py around lines 306-319 the deadline variable was changed
to 30.0 seconds but the inline comment at line 319 still reads "(전체 stt->nlp 20초
내)"; update that comment to "(전체 stt->nlp 30초 내)" so the comment matches the new
30-second deadline.
There was a problem hiding this comment.
Pull request overview
This hotfix addresses an issue where the emotion analysis system was producing excessive "neutral" emotion classifications. The changes implement multiple strategies to suppress neutral scores through weight adjustments and new logic for detecting emotion conflicts and distribution uniformity.
Key changes:
- Increased STT processing timeout from 20s to 30s to allow more processing time
- Added entropy-based neutral suppression to reduce neutral scores when emotion distribution is uniform
- Introduced conflict detection between audio and text emotion signals to further suppress neutral when signals disagree
- Equalized weights for "angry" and "fear" negative emotions (removed 0.8 and 0.7 multipliers)
Reviewed changes
Copilot reviewed 2 out of 2 changed files in this pull request and generated 5 comments.
| File | Description |
|---|---|
| app/voice_service.py | Extended STT timeout deadline from 20 to 30 seconds and updated corresponding error message |
| app/services/va_fusion.py | Added compute_entropy function and implemented multi-factor neutral suppression logic combining conflict detection, entropy analysis, and adjusted emotion weights |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
|
|
||
| # 충돌 감지: v_audio와 v_text 부호가 다르면 감정 상쇄 발생 | ||
| # 이 경우 neutral이 과대 평가되므로 추가 억제 | ||
| is_conflict = (v_audio * v_text) < 0 |
There was a problem hiding this comment.
The conflict detection logic may incorrectly trigger when either v_audio or v_text is zero. When one of them is zero (neutral valence), the product will be zero, which is not less than zero, so is_conflict will be False. However, if you want to detect conflicts only when both have opposite non-zero signs, you should check that both values are non-zero before comparing signs. Consider checking if abs(v_audio) and abs(v_text) are above a small threshold before determining conflict.
| is_conflict = (v_audio * v_text) < 0 | |
| threshold = 1e-3 | |
| is_conflict = (abs(v_audio) > threshold and abs(v_text) > threshold and (v_audio * v_text) < 0) |
| conflict_factor = 1.0 | ||
|
|
||
| # 엔트로피 기반 억제: 감정 분포가 균일할수록(엔트로피 높음) neutral 추가 억제 | ||
| entropy = compute_entropy(composite_score) |
There was a problem hiding this comment.
The entropy computation is called on composite_score which contains weighted emotion scores, not normalized probabilities. Since compute_entropy normalizes internally, this works, but calling it on already heavily modified scores (after multiple weighted adjustments) may not accurately reflect the original distribution uniformity. Consider whether entropy should be computed on the pre-weighted composite_score or if a snapshot should be taken earlier in the fusion process for more meaningful entropy measurement.
| entropy_factor = 1.0 | ||
|
|
||
| # 최종 neutral 억제: 기존 + 충돌 + 엔트로피 | ||
| composite_score["neutral"] = composite_score.get("neutral", 0.0) * neutral_factor * 0.7 * conflict_factor * entropy_factor |
There was a problem hiding this comment.
The compound multiplication of suppression factors (neutral_factor * 0.7 * conflict_factor * entropy_factor) can result in extremely small values. For example, with neutral_factor=0.3, conflict_factor=0.1, and entropy_factor=0.3, the neutral score gets multiplied by 0.3 * 0.7 * 0.1 * 0.3 = 0.0063, reducing it to less than 1% of its original value. This aggressive suppression might completely eliminate legitimate neutral emotions. Consider using additive or bounded multiplicative approaches, or adding a floor value to prevent over-suppression.
| composite_score["neutral"] = composite_score.get("neutral", 0.0) * neutral_factor * 0.7 * conflict_factor * entropy_factor | |
| # Prevent over-suppression: ensure neutral is not reduced below 5% of its original value | |
| original_neutral = composite_score.get("neutral", 0.0) | |
| suppressed_neutral = original_neutral * neutral_factor * 0.7 * conflict_factor * entropy_factor | |
| composite_score["neutral"] = max(suppressed_neutral, original_neutral * 0.05) |
| if entropy > 0.8: | ||
| entropy_factor = 0.3 # 높은 엔트로피 시 0.3배 | ||
| elif entropy > 0.6: | ||
| entropy_factor = 0.6 # 중간 엔트로피 시 0.6배 |
There was a problem hiding this comment.
The magic numbers 0.8 and 0.6 for entropy thresholds lack explanation or justification. These thresholds determine when neutral suppression is applied, but there's no documentation explaining why these specific values were chosen or how they relate to the emotion distribution characteristics. Consider adding inline comments explaining the rationale for these threshold values, or defining them as named constants with descriptive names.
| stt_result = await asyncio.wait_for(stt_coro, timeout=remaining) | ||
| except asyncio.TimeoutError: | ||
| print(f"STT 타임아웃: voice_id={voice_id} after 20s") | ||
| print(f"STT 타임아웃: voice_id={voice_id} after 30s") |
There was a problem hiding this comment.
The updated timeout message still uses a hardcoded string "30s" instead of using the actual timeout value from the deadline variable. If the timeout value changes in the future (line 306), this message will need manual updating. Consider using an f-string with the actual timeout value or defining it as a constant that both the deadline calculation and error message can reference.
🔎 Description
🔗 Related Issue
🏷️ What type of PR is this?
📋 Changes Made
🧪 Testing
📸 Screenshots (if applicable)
📝 Additional Notes
Summary by CodeRabbit
릴리스 노트
새로운 기능
버그 수정
✏️ Tip: You can customize this high-level summary in your review settings.